Deflake limitrange singleflight test #113736
Conversation
k8s-ci-robot
added
the
release-note-none
k8s-ci-robot
added
kind/flake
|
@liggitt: GitHub didn't allow me to request PR reviews from the following users: aimuz. Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
cncf-cla: yes
|
@liggitt: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-priority
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/sig node |
k8s-ci-robot
added
sig/node
| // he always blocking before sending the signal | ||
| <-unhold | ||
| // make the handler slow so concurrent calls exercise the singleflight | ||
| time.Sleep(time.Second) |
There was a problem hiding this comment.
this has to be always higher than the time it takes to create the goroutines and hit the group
There was a problem hiding this comment.
no, if it is not higher, the ones that missed the singleflight hit the cache, and the test still passes
There was a problem hiding this comment.
I think that the concurrency pattern we want to implement is the one provided by sync.Cond, so we put all goroutines go to sleep here and we wake them up later ... but this approach is easier to read and in my local tests it seems very reliable
7m40s: 7249 runs so far, 0 failures
7m45s: 7344 runs so far, 0 failures
7m50s: 7440 runs so far, 0 failures
7m55s: 7488 runs so far, 0 failures
8m0s: 7584 runs so far, 0 failures
8m5s: 7680 runs so far, 0 failures
8m10s: 7728 runs so far, 0 failures
8m15s: 7824 runs so far, 0 failures
8m20s: 7920 runs so far, 0 failures
8m25s: 7968 runs so far, 0 failures
8m30s: 8064 runs so far, 0 failures
8m35s: 8157 runs so far, 0 failures
8m40s: 8208 runs so far, 0 failures
```
/lgtm
|
/lgtm |
What type of PR is this?
/kind flake
What this PR does / why we need it:
Deflakes the limit range test in case of slow CI
Alternative to #113378
Which issue(s) this PR fixes:
Fixes #113377
/cc @aojea @aimuz